Add persistent closed channel history and list_closed_channels()#882
Add persistent closed channel history and list_closed_channels()#882f3r10 wants to merge 1 commit intolightningdevkit:mainfrom
list_closed_channels()#882Conversation
Introduce `ClosedChannelDetails`, a new record type persisted to the KV store under the `"closed_channels"` namespace whenever a channel closes. Records are written in the `ChannelClosed` event handler and loaded back at startup in parallel with other stores via `tokio::join!`. Add `Node::list_closed_channels()` to expose the full history of closed channels across restarts. Track outbound channel direction via an in-memory `outbound_channel_ids` set seeded from `channel_manager.list_channels()` at startup and updated on `ChannelPending` events, since `ChannelClosed` does not carry that information directly.
|
I've assigned @tnull as a reviewer! |
Camillarhi
left a comment
Camillarhi
left a comment
There was a problem hiding this comment.
Thanks for looking into this. I dropped some comments. Also, CI seems to be failing due to a formatting issue. Kindly run cargo fmt --all to resolve this
| closed_at: closed_at.0.ok_or(DecodeError::InvalidValue)?, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Can we use impl_writeable_tlv_based! here instead? Matches the rest of the StorableObject types in the codebase
| /// Retrieve a list of closed channels. | ||
| /// | ||
| /// Channels are added to this list when they are closed and will be persisted across restarts. | ||
| pub fn list_closed_channels(&self) -> Vec<ClosedChannelDetails> { |
There was a problem hiding this comment.
The new list_closed_channels() API isn't exposed in bindings.
| /// [`Node::list_closed_channels`]: crate::Node::list_closed_channels | ||
| #[derive(Clone, Debug, PartialEq, Eq)] | ||
| #[cfg_attr(feature = "uniffi", derive(uniffi::Record))] | ||
| pub struct ClosedChannelDetails { |
There was a problem hiding this comment.
ClosedChannelDetails isn't exposed in bindings
|
🔔 1st Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 3rd Reminder Hey @tnull! This PR has been waiting for your review. |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Thanks, already looks pretty good. Some comments.
| let mut set = tokio::task::JoinSet::new(); | ||
|
|
||
| // Fill JoinSet with tasks if possible | ||
| while set.len() < BATCH_SIZE && !stored_keys.is_empty() { |
There was a problem hiding this comment.
Makes sense to DRY this up for now, but depending on when #876 lands we might need to adapt this to use the new BatchingStore.
| assert_eq!(record.funding_txo.unwrap().txid, funding_txo.txid); | ||
| assert!(record.closure_reason.is_some()); | ||
|
|
||
| closed_channel_before = record.clone(); |
| node_b.sync_wallets().unwrap(); | ||
|
|
||
| // Verify the record is present before restart. | ||
| let closed_a = node_a.list_closed_channels(); |
There was a problem hiding this comment.
Should we also check that b also persisted the closed channel?
| user_channel_id: UserChannelId, | ||
| /// The `node_id` of the channel counterparty. | ||
| /// | ||
| /// This will be `None` for events serialized by LDK Node v0.1.0 and prior. |
There was a problem hiding this comment.
I think we should be good breaking backwards compat with LDK node v0.1, so let's make this a required field. Then we can also avoid having the Option propagate. Please add a not regarding serialization compatibility as part of a # Pending section in CHANGELOG.md
| /// [`Node::list_closed_channels`]: crate::Node::list_closed_channels | ||
| #[derive(Clone, Debug, PartialEq, Eq)] | ||
| #[cfg_attr(feature = "uniffi", derive(uniffi::Record))] | ||
| pub struct ClosedChannelDetails { |
There was a problem hiding this comment.
Might be good also recording whether this was an announced channel.
4 participants
Summary
Closes #851.
Adds persistent historical closed channel records so applications can query all channels that have ever closed on this node, surviving restarts.
New public API
Node::list_closed_channels() -> Vec<ClosedChannelDetails>ClosedChannelDetailsstruct:channel_id,user_channel_id,counterparty_node_id,funding_txo,channel_capacity_sats,last_local_balance_msat,is_outbound,closure_reason,closed_atEvent::ChannelClosedgains three new fields:channel_capacity_sats,channel_funding_txo,last_local_balance_msatImplementation
Persistence (
src/closed_channel.rs,src/io/):ClosedChannelDetailsimplementsStorableObjectwith insert-only semantics (closed records are immutable). Records are stored under the"closed_channels"KV namespace keyed byUserChannelId.Startup (
src/builder.rs):read_closed_channels()joins the existingtokio::join!block so historical records are loaded in parallel with payments, pending payments, and node metrics — no added sequential startup cost as history gronws.Event handlinng (
src/event.rs): OnChannelClosed, the handler writes aClosedChannelDetailsrecord to the store and emits the enriched public event. Channel direction (is_outbound) is tracked via an in-memoryoutbound_channel_idsset, seeded fromchannel_manager.list_channels()at startup and updated onChannelPending, sinceChannelCloseddoes not carry that information directly.